packages/raystack/components/data-view/data-view.module.css-1-20 (1)

1-20: ⚠️ Potential issue | 🟡 Minor

Stylelint declaration-empty-line-before violations.

• Line 5: remove the empty line before border-bottom inside .toolbar.
• Line 18: add an empty line before padding (after the --select-width custom property) in .display-popover-properties-container.

✏️ Proposed fix

 .toolbar {
   padding: var(--rs-space-3) 0;
   align-self: stretch;
-
   border-bottom: 0.5px solid var(--rs-color-border-base-primary);
   background: var(--rs-color-background-base-primary);
 }
@@
 .display-popover-properties-container {
   /* Todo: var does not exist for 160px */
   --select-width: 160px;
+
   padding: var(--rs-space-5);
   border-bottom: 1px solid var(--rs-color-border-base-primary);
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/data-view.module.css` around lines 1 -
20, Fix Stylelint declaration-empty-line-before violations in
data-view.module.css: inside the .toolbar block (symbol .toolbar) remove the
blank line before the border-bottom declaration so there is no empty line
between align-self and border-bottom; inside
.display-popover-properties-container (symbol
.display-popover-properties-container) add a single empty line after the custom
property --select-width and before the padding declaration so there is exactly
one blank line between them to satisfy the rule.

packages/raystack/components/data-view/hooks/useDataView.tsx-8-10 (1)

8-10: ⚠️ Potential issue | 🟡 Minor

Error message references nonexistent DataView.Provider.

The context is supplied by rendering <DataView> (see packages/raystack/components/data-view/data-view.tsx lines 197–230), not a DataView.Provider sub-component. The message will mislead consumers debugging the error.

✏️ Suggested wording

-    throw new Error('useDataView must be used inside of a DataView.Provider');
+    throw new Error('useDataView must be used inside of a <DataView> component');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/hooks/useDataView.tsx` around lines 8
- 10, The thrown Error in the useDataView hook references a nonexistent
"DataView.Provider"; update the message to refer to the actual provider
component (e.g., "<DataView>") so consumers aren't misled: find the useDataView
function where ctx is checked (variable ctx) and replace the Error text to
something like "useDataView must be used inside of a <DataView> component" or
similar clear wording.

packages/raystack/components/data-view/components/toolbar.tsx-19-47 (1)

19-47: ⚠️ Potential issue | 🟡 Minor

shouldShowFilters gates the entire Toolbar, hiding user-provided children too.

When a consumer composes their own toolbar (e.g., <Toolbar><Search/>...other...</Toolbar>), the whole container is suppressed if shouldShowFilters is false. That flag is conceptually about whether to show the default filter chips; it shouldn't suppress arbitrary children like Search or custom controls. Also, the two render branches duplicate the wrapping Flex.

♻️ Suggested consolidation

-  const { shouldShowFilters = false } = useDataView<TData>();
-
-  if (!shouldShowFilters) {
-    return null;
-  }
-
-  // If children are provided, render them so consumers can compose Search / Filters / DisplayControls.
-  if (children) {
-    return (
-      <Flex
-        className={cx(styles['toolbar'], className)}
-        justify='between'
-        align='center'
-      >
-        {children}
-      </Flex>
-    );
-  }
-
-  return (
-    <Flex
-      className={cx(styles['toolbar'], className)}
-      justify='between'
-      align='center'
-    >
-      <Filters<TData> />
-      <DisplaySettings<TData> />
-    </Flex>
-  );
+  const { shouldShowFilters = false } = useDataView<TData>();
+
+  // Only gate the default composition; honour user-provided children unconditionally.
+  if (!children && !shouldShowFilters) {
+    return null;
+  }
+
+  return (
+    <Flex
+      className={cx(styles['toolbar'], className)}
+      justify='between'
+      align='center'
+    >
+      {children ?? (
+        <>
+          <Filters<TData> />
+          <DisplaySettings<TData> />
+        </>
+      )}
+    </Flex>
+  );

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/toolbar.tsx` around lines
19 - 47, The Toolbar currently returns null when useDataView().shouldShowFilters
is false which also hides consumer-provided children; change Toolbar (in
toolbar.tsx) to stop gating the whole component on shouldShowFilters: always
render the Flex wrapper (cx(styles['toolbar'], className), justify='between',
align='center') when children exist so custom Search/controls are not
suppressed, and consolidate the duplicate Flex branches into a single return;
for the default content, only render <Filters<TData> /> when shouldShowFilters
is true and always render <DisplaySettings<TData> /> as before when there are no
children.

packages/raystack/components/data-view/hooks/useFilters.tsx-18-24 (1)

18-24: ⚠️ Potential issue | 🟡 Minor

Default value for multiselect filter type falls through to ''.

For FilterType.multiselect, operators are in/notin which expect an array value, but the ternary falls through to the empty-string branch. This will yield an initial filter with value: '' that downstream operator handling (notin/in) will likely mis-evaluate or crash on.

🛡️ Suggested fix

     const defaultValue =
       field.defaultFilterValue ??
       (filterType === FilterType.date
         ? new Date()
-        : filterType === FilterType.select
-          ? options[0]?.value
-          : '');
+        : filterType === FilterType.select
+          ? options[0]?.value
+          : filterType === FilterType.multiselect
+            ? []
+            : '');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/hooks/useFilters.tsx` around lines 18
- 24, The defaultValue ternary in useFilters.tsx incorrectly falls through
FilterType.multiselect to an empty string; change the expression that computes
defaultValue (the variable named defaultValue) to handle FilterType.multiselect
explicitly and return an array (e.g., [] or a sensible default from options like
[options[0]?.value] or options.map(...)) instead of ''. Update the branch that
checks filterType so FilterType.multiselect yields an array value compatible
with the 'in'/'notin' operators, leaving existing handling for FilterType.date
and FilterType.select unchanged.

apps/www/src/app/examples/dataview/page.tsx-515-520 (1)

515-520: ⚠️ Potential issue | 🟡 Minor

Fix the stale DataView example link.

This page is served from /examples/dataview, but the sidebar links the DataView item to /examples/dataview-list, which looks stale and can navigate users away from the current example.

🔗 Proposed fix

           <Sidebar.Item
-            href='/examples/dataview-list'
+            href='/examples/dataview'
             leadingIcon={<SidebarIcon />}
           >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/www/src/app/examples/dataview/page.tsx` around lines 515 - 520,
Sidebar.Item for the DataView example is linking to the stale path
'/examples/dataview-list'; update the href in the Sidebar.Item that renders
"DataView · People" (the Sidebar.Item with leadingIcon={<SidebarIcon />}) to
point to the current page path '/examples/dataview' so the sidebar does not
navigate away from the served example.

packages/raystack/components/data-view/components/filters.tsx-81-154 (1)

81-154: ⚠️ Potential issue | 🟡 Minor

Wire classNames.addFilter or remove it from the public props.

classNames.addFilter is exposed but never applied, so consumers cannot style the add-filter control as advertised.

🎨 Proposed fix

 interface AddFilterProps<TData> {
   fieldList: DataViewField<TData>[];
   appliedFiltersSet: Set<string>;
   onAddFilter: (field: DataViewField<TData>) => void;
+  className?: string;
   children?: Trigger<TData>;
 }

 function AddFilter<TData>({
   fieldList = [],
   appliedFiltersSet,
   onAddFilter,
+  className,
   children
 }: AddFilterProps<TData>) {
@@
-  return availableFilters.length > 0 ? (
+  return availableFilters.length > 0 ? (
+    <span className={className}>
     <Menu>
       <Menu.Trigger
         render={isValidElement(trigger) ? trigger : <button>{trigger}</button>}
       />
@@
-    </Menu>
+    </Menu>
+    </span>
   ) : null;
 }
@@
       <AddFilter
         fieldList={filterableFields}
         appliedFiltersSet={appliedFiltersSet}
         onAddFilter={onAddFilter}
+        className={classNames?.addFilter}
       >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/filters.tsx` around lines
81 - 154, The Filters component currently exposes classNames.addFilter but never
uses it; either apply that class to the AddFilter control or remove it from the
public props. Fix by updating the Filters function to pass classNames?.addFilter
into the AddFilter component (e.g., AddFilter className or appropriate prop) so
consumers can style the add-filter control, or alternatively remove addFilter
from the classNames type and all references to it in Filters and its types if we
don't intend to support it. Target the Filters component, the
classNames.addFilter property, and the AddFilter usage to implement the change.

packages/raystack/components/data-view/components/search.tsx-53-59 (1)

53-59: ⚠️ Potential issue | 🟡 Minor

Normalize the search input value to prevent controlled/uncontrolled switching.

value={tableQuery?.search} passes undefined initially, then a string after typing. Use the nullish coalescing operator to keep the input always controlled:

Proposed fix

       {...props}
       onChange={handleSearch}
-      value={tableQuery?.search}
+      value={tableQuery?.search ?? ''}
       onClear={handleClear}
       disabled={isDisabled}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/search.tsx` around lines 53
- 59, The Search input is switching between uncontrolled and controlled because
value={tableQuery?.search} can be undefined; update the JSX in the Search
component usage to always provide a string (e.g., default to an empty string) so
the input remains controlled — change the value prop expression where Search is
rendered (refer to Search, tableQuery?.search, handleSearch, handleClear) to use
a nullish-coalescing fallback (empty string) and ensure downstream handlers
accept an empty string.

docs/rfcs/002-unified-dataview-component.md-421-428 (1)

421-428: ⚠️ Potential issue | 🟡 Minor

Remove or replace the local worktree reference.

The RFC links to .claude/worktrees/dataview/ANALYSIS.md, which is not a stable repo document and appears to reference removed/local analysis. This will be stale for readers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/rfcs/002-unified-dataview-component.md` around lines 421 - 428, The RFC
currently links to a local worktree `.claude/worktrees/dataview/ANALYSIS.md`,
which is not a stable repo document and should be removed or replaced; update
the Helpful Links section in docs/rfcs/002-unified-dataview-component.md by
deleting that local path and either replace it with a stable, committed document
(e.g., an internal RFC appendix, a committed ANALYSIS.md in the repo, or a
summarized bullet) or remove the entry entirely so the RFC only references
public/stable resources.

packages/raystack/components/data-view/components/virtualized-content.tsx-305-316 (1)

305-316: ⚠️ Potential issue | 🟡 Minor

Render loader rows for initial virtualized loading.

When isLoading is true and rows.length is 0, hasData is true, but showLoaderRows is false and the virtualizer count is 0, so the body renders blank instead of showing skeletons.

Also applies to: 373-427

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/virtualized-content.tsx`
around lines 305 - 316, The component currently sets showLoaderRows to isLoading
&& rows.length > 0 and passes rows.length as the virtualizer count, so when
isLoading is true but rows.length === 0 (initial load with hasData true) the
virtualized list is empty; change showLoaderRows to isLoading && (rows.length >
0 || hasData) and pass a non‑zero fallback count to useVirtualizer (e.g.,
useVirtualizer count = showLoaderRows ? defaultSkeletonCount : rows.length) so
the virtualizer will render skeleton rows during initial loading; ensure
estimateSize and the row rendering logic handle those loader rows by treating
them as regular rows with rowHeight or groupHeaderHeight as needed.

packages/raystack/components/data-view/utils/index.tsx-120-146 (1)

120-146: ⚠️ Potential issue | 🟡 Minor

Avoid reference equality for filter values.

Line 144 compares raw values with !==, so recreated arrays/objects such as multi-select in values will look changed even when semantically identical.

Proposed fix

+const normalizeFilterValue = (value: unknown): string => {
+  if (Array.isArray(value)) {
+    return JSON.stringify([...value].sort());
+  }
+  return JSON.stringify(value);
+};
+
 const generateFilterMap = (
   filters: InternalFilter[] = []
-): Map<string, any> => {
+): Map<string, string> => {
   return new Map(
     filters
       ?.filter(data => data._type === FilterType.select || data.value !== '')
-      .map(({ name, operator, value }) => [`${name}-${operator}`, value])
+      .map(({ name, operator, value }) => [
+        `${name}-${operator}`,
+        normalizeFilterValue(value)
+      ])
   );
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/utils/index.tsx` around lines 120 -
146, The isFilterChanged function incorrectly uses reference equality when
comparing filter values; replace the raw !== comparison with a deep equality
check (e.g., lodash/isEqual or a small deepEqual helper) so arrays/objects (like
multi-select "in" values) compare by content; update isFilterChanged to
import/use isEqual (or implement deepEqual) and change the comparison in the
[...newFilterMap].some callback from oldFilterMap.get(key) !== value to
!isEqual(oldFilterMap.get(key), value), ensuring null/undefined cases are
handled consistently.

packages/raystack/components/data-view/components/grouping.tsx (1)

29-32: Minor: search the filtered groupableFields instead of fields.

Since the Select only exposes groupableFields + the default option, searching fields works today, but tightening the scope makes the invariant self-evident and prevents accidentally accepting a non-groupable accessor if a caller invokes the handler directly.

-    const field = fields.find(f => f.accessorKey === fieldAccessor);
+    const field = groupableFields.find(f => f.accessorKey === fieldAccessor);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/components/grouping.tsx` around lines
29 - 32, In the grouping.tsx handler the lookup uses fields.find(...) but should
search the filtered groupableFields to enforce the Select's invariant; update
the lookup to use groupableFields.find(f => f.accessorKey === fieldAccessor) and
then call onChange(field.accessorKey) only when that found item exists
(preserving the existing conditional); this ensures fieldAccessor is validated
against groupableFields rather than the full fields list (refer to symbols:
fieldAccessor, fields, groupableFields, onChange).

packages/raystack/components/data-view/data-view.module.css (1)

11-17: TODO: missing design tokens for 300px / 160px.

Two Todo: var does not exist comments hard-code values that should round-trip through design tokens. Worth tracking so the component stays consistent once tokens are added.

Want me to open an issue to add the missing spacing tokens and swap these hard-coded values?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/data-view.module.css` around lines 11
- 17, Replace the hard-coded sizing with design-token CSS variables: change the
min-width: 300px occurrence to use the appropriate spacing token var(--space-??)
(or var(--token-spacing-300) if your token naming differs) with a pixel fallback
like min-width: var(--token-spacing-300, 300px), and change --select-width:
160px inside .display-popover-properties-container to use the matching token
var(--token-spacing-160, 160px); remove the "Todo: var does not exist" comments
and, if the tokens don't yet exist, open an issue to add spacing tokens for
these sizes and reference the selectors/minified properties (min-width and
--select-width on .display-popover-properties-container) so the tokens can be
added and updated later.

packages/raystack/components/data-view/index.ts (1)

6-18: Consider using type-only re-exports for the DataView types as a best practice.

All exported symbols (DataViewField, DataViewFilter, DataViewListColumn, DataViewListProps, DataViewProps, DataViewQuery, DataViewSort, DataViewTableColumn, DataViewTableProps, InternalFilter, InternalQuery) are confirmed as type/interface declarations. While your current TypeScript configuration does not require it, using export type clarifies intent and prevents accidental value usage:

♻️ Proposed export cleanup

-export {
+export type {
   DataViewField,
   DataViewFilter,
   DataViewListColumn,
   DataViewListProps,
   DataViewProps,
   DataViewQuery,
   DataViewSort,
   DataViewTableColumn,
   DataViewTableProps,
   InternalFilter,
   InternalQuery
 } from './data-view.types';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/raystack/components/data-view/index.ts` around lines 6 - 18, All
listed exports from './data-view.types' are type/interface-only; change the
re-export to use type-only exports to make intent explicit and prevent
accidental value imports — replace the current export list with type-only
re-exports for DataViewField, DataViewFilter, DataViewListColumn,
DataViewListProps, DataViewProps, DataViewQuery, DataViewSort,
DataViewTableColumn, DataViewTableProps, InternalFilter, and InternalQuery
(i.e., use "export type { ... }" for those symbols).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes